chore: add pre-commit hooks and commitlint configuration #14
Conversation
WalkthroughThis pull request introduces two new configuration files to establish automated code quality and commit message validation workflows. The .pre-commit-config.yaml file defines a suite of pre-commit hooks spanning file validation, code formatting, linting, spell checking, and security scanning with specified versions and options. The commitlint.config.ts file configures commit message validation rules following the conventional commits standard. No existing application code is modified. Estimated Code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
commitlint.config.mjs (2)
33-41: Avoid redundant checks: pick eithersigned-off-byortrailer-existsBoth rules enforce the same trailer; keeping both is redundant. Prefer a single rule (usually
trailer-existsto ensure it’s in the trailer section)."subject-full-stop": [2, "never", "."], - "signed-off-by": [2, "always", "Signed-off-by:"], + // "signed-off-by": [2, "always", "Signed-off-by:"], /* * Enable type if we need, warn it currently. */ "type-enum": [0, "always", ['ci', 'docs', 'feat', 'fix', 'perf', 'refactor', 'revert', 'rfe', 'style']], "type-case": [0, "always", "lower-case"], "type-empty": [0, "never"], "trailer-exists": [2, "always", "Signed-off-by:"]Reference list of supported rules includes both entries. (commitlint.js.org)
37-37: Comment says “warn it currently”, but severity is off (0)If you want warnings (non‑blocking), set severity to 1.
- "type-enum": [0, "always", ['ci', 'docs', 'feat', 'fix', 'perf', 'refactor', 'revert', 'rfe', 'style']], + "type-enum": [1, "always", ['ci', 'docs', 'feat', 'fix', 'perf', 'refactor', 'revert', 'rfe', 'style']],Committed types are enforced by
type-enumwhen enabled. (commitlint.js.org).pre-commit-config.yaml (2)
15-16: FYI:no-commit-to-branchalready protectsmainandmasterby defaultYou can add more branches/patterns via
argsif needed; otherwise defaults are fine.Docs for defaults and args are here. (github.com)
35-41: Optional: pin commitlint dependencies for stricter reproducibilityYou already pin the hook revision. To avoid potential
@commitlint/*version mismatches when adding plugins later, consider pinning versions inadditional_dependencies(e.g.,@commitlint/[email protected]). Not required now, just future‑proofing.Background on dependency mismatches and rules recognition. (commitlint.js.org)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)commitlint.config.mjs(1 hunks)
🔇 Additional comments (5)
commitlint.config.mjs (3)
6-11: Config shape and ESM export look goodUsing ESM with
export default, extending@commitlint/config-conventional, and settingformatter: '@commitlint/format'matches current commitlint docs. (commitlint.js.org)
49-49: Do you want to enforce Signed-off-by on merge commits too?With
defaultIgnores: true, commitlint skips merge commits; that meansSigned-off-by:won’t be required on merges. If you want it enforced everywhere, setdefaultIgnores: false(and add customignoresif needed).Docs detail
ignoresanddefaultIgnores. (commitlint.js.org)
53-55: Nice touch onhelpUrlLinking to Avocado’s commit style guide matches the local policy (72 char header, no trailing period, Signed‑off‑by).
Avocado commit style guide confirms these rules. (avocado-framework.readthedocs.io)
.pre-commit-config.yaml (2)
1-1: Correct staging setup for commitlintInstalling both
pre-commitandcommit-msghook types is the recommended pattern. (github.com)
3-17: Hook versions look currentPinned versions are recent/stable:
pre-commit-hooks v6.0.0,gitleaks v8.28.0,isort 6.0.1,black 25.9.0,codespell v2.4.1,commitlint-pre-commit-hook v9.22.0.Refs: pre-commit-hooks v6.0.0 usage; Gitleaks v8.28.0 release; isort 6.0.1; Black 25.9.0; Codespell 2.4.1; commitlint-pre-commit-hook v9.22.0. (github.com)
richtja
left a comment
richtja
left a comment
There was a problem hiding this comment.
Hi @PaulYuuu, thank you for introducing the pre-commit hooks, this will definitely be an improvement of our static-checks. I have just couple of comments:
- IIUIC this should replace the current static-checks scripts, therefore I would remove them from repo so we won't have two solutions.
- The last missing static check is lint check. Unfortunatly that is higly configurable in static-checks therefore I don't think we can use regular lint hook for it and will need to run the current script via hooks. Something like this:
- repo: local
hooks:
- id: check-lint
name: Run pylint checks
entry: static-checks/check-lint
language: system
types: [python]
pass_filenames: false
.pre-commit-config.yaml
Outdated
| args: ['--maxkb=5120'] | ||
| - id: check-ast | ||
| - id: check-yaml | ||
| exclude: .cirrus.yml |
There was a problem hiding this comment.
The .cirrus.yml is only related to avocado-vt. For example, on avocado file we need to exclude different files for example examples/yaml_to_mux/. If we won't be able to have a separated exclude list for different repos then I think we will have to remove this check.
There was a problem hiding this comment.
Yes, cirrus.yml does not follow the yaml syntax, but it's vt only. So I am okay to remove it from here, if we have this requirement at specific repository, we can use another github action or static-check.
Hello @richtja, I did not add pylint here is because the scope of pylint is not clear for me and I am not sure if we still need it, as we already have black and isort. I can also add pylint pre-commit hook, and for each repo, link the pylintrc from static-check submodule to the repo root, then it should works. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
commitlint.config.ts (1)
8-10: Consider removing disabled rules for clarity.Multiple rules are explicitly disabled (level
0). If these rules are intentionally not enforced, consider removing them entirely from the configuration to reduce clutter and make the active rules more apparent. If they serve as documentation of available options, this is acceptable but may confuse contributors.Apply this diff to remove the disabled rules:
// Body 'body-leading-blank': [2, 'always'], - 'body-empty': [0, 'never'], - 'body-min-length': [0, 'always', 1], - 'body-case': [0, 'always', 'lower-case'], // Footer 'footer-leading-blank': [2, 'always'], - 'footer-empty': [0, 'never'], - 'footer-max-length': [0, 'always', 72], // Header - 'header-case': [0, 'always', 'lower-case'], 'header-full-stop': [2, 'never', '.'], 'header-max-length': [2, 'always', 72], 'header-min-length': [2, 'always', 1], // Scope / Subject - 'scope-case': [0, 'always', 'lower-case'], - 'subject-case': [0, 'always', 'lower-case'], - 'subject-empty': [0, 'never'], 'subject-full-stop': [2, 'never', '.'],Also applies to: 14-15, 18-18, 24-26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)commitlint.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:59:46.902Z
Learnt from: PaulYuuu
PR: avocado-framework/avocado-static-checks#14
File: .pre-commit-config.yaml:30-35
Timestamp: 2025-09-29T03:59:46.902Z
Learning: When .pre-commit-config.yaml is in a submodule, it's correct for it to reference files like 'spell.ignore' that will exist in the consuming repositories, not in the submodule itself.
Applied to files:
.pre-commit-config.yaml
🔇 Additional comments (2)
commitlint.config.ts (1)
1-36: LGTM! Configuration correctly enforces commit conventions.The TypeScript configuration properly extends the conventional preset and enforces the key requirements mentioned in the PR objectives: signed-off-by trailer (Line 30) and conventional commit format with appropriate header constraints (Lines 19-21, 27). The structure and typing are correct, and the helpUrl appropriately points to the contributor style guide.
.pre-commit-config.yaml (1)
1-45: Comprehensive pre-commit configuration with proper commitlint integration.The configuration provides a well-rounded set of hooks covering code quality (black, isort, pylint), security (gitleaks, detect-private-key), file validation (check-yaml, check-ast), and commit message validation (commitlint). The commitlint hook correctly references the commitlint.config.ts file and uses the appropriate commit-msg stage.
Key strengths:
- Lines 40-45: Commitlint integration properly adds @commitlint/config-conventional dependency and will auto-discover commitlint.config.ts
- Line 26: isort configured with black profile ensures formatter compatibility
- Lines 7-8: Large file check with 5MB limit prevents accidental commits of binaries
- Lines 14-15: Line ending normalization enforces LF across the codebase
Based on learnings: The spell.ignore reference (Line 39) is correct since this is a submodule configuration and the file will exist in consuming repositories.
.pre-commit-config.yaml
Outdated
| - repo: https://github.com/pylint-dev/pylint | ||
| rev: v4.0.0 | ||
| hooks: | ||
| - id: pylint |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Clarify pylint configuration approach for consuming repositories.
The pylint hook has no arguments or configuration specified. Based on the PR comments, you mentioned that pylint's scope is unclear and that you would link each repo's pylintrc from the static-checks submodule to the repo root. Please verify this approach will work correctly, as pylint typically requires a configuration file to function properly.
Consider one of the following approaches:
- Add a default pylintrc to this submodule that consuming repos can override
- Document in a README how consuming repos should configure their pylintrc
- Add
args: ['--rcfile=pylintrc']to make the expected configuration file explicit
Run this script to verify how other Python projects in the avocado-framework organization handle pylint configuration:
🏁 Script executed:
#!/bin/bash
# Search for pylint configuration files across avocado-framework repositories
gh repo list avocado-framework --limit 50 --json name,url | \
jq -r '.[] | select(.name | startswith("avocado")) | .name' | \
while read repo; do
echo "=== Checking $repo ==="
gh api "repos/avocado-framework/$repo/contents" --jq '.[] | select(.name | test("pylintrc|.pylintrc|pyproject.toml|setup.cfg")) | .name'
doneLength of output: 398
Provide a default pylintrc and explicit rcfile arg for the pylint hook
The submodule currently lacks a pylintrc (only the main avocado repo has one), so add a pylintrc here and update the pylint hook to reference it via --rcfile. Alternatively, document in a README how consuming repos should supply or override their own pylintrc.
🤖 Prompt for AI Agents
In .pre-commit-config.yaml around lines 31 to 34, the pylint hook is referencing
the pylint repo but there is no pylintrc in this submodule; add a default
pylintrc file into the submodule (e.g., at .pylintrc) and update the pylint hook
to pass the explicit rcfile argument (--rcfile=.pylintrc) so the hook uses the
local config, or alternatively add a README entry explaining that consuming
repos must provide/override a pylintrc and modify the hook to require an
explicit path via configuration.
Hi @PaulYuuu, so the pylint still makes sense in here because it adds much more checks than black or isort. Unfortunately IMO we can't use the pylint pre-commit hook directly, and we need to add the current static-checks/check-lint script into the pre-commit hooks, because in projects like Avocado or AAutils, we use different pylintrc files for different parts of each repo and the check-lint script helps us with that. |
OK, thank you for the explanation, it makes senses for me as I can see aautils has the config so I have an idea that still using the pylint hook, but we can modify the entry as pre-commit support this, like I will try if this can meet our requirement, but this request the avocado-static-check be a submodule should correct the name in each repo(currently aautils uses static-check, vt uses avocado-static-check), or just link the check-lint script from submodule to repo root. |
I see, and I think we can update the submodule name in each repo, it shouldn't be a problem. |
PaulYuuu
force-pushed
the
pre-commit
branch
4 times, most recently
from
d89c747 to
411d125
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)commitlint.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- commitlint.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:59:46.902Z
Learnt from: PaulYuuu
PR: avocado-framework/avocado-static-checks#14
File: .pre-commit-config.yaml:30-35
Timestamp: 2025-09-29T03:59:46.902Z
Learning: When .pre-commit-config.yaml is in a submodule, it's correct for it to reference files like 'spell.ignore' that will exist in the consuming repositories, not in the submodule itself.
Applied to files:
.pre-commit-config.yaml
PaulYuuu
force-pushed
the
pre-commit
branch
3 times, most recently
from
424986f to
8b79cf7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
commitlint.config.ts (1)
5-28: Remove disabled rules for clarity.Multiple rules are explicitly set to level 0 (disabled), which adds noise without changing behavior. Commitlint best practice is to only include rules that override the preset defaults.
Consider removing the disabled rules:
extends: ['@commitlint/config-conventional'], rules: { // Body 'body-leading-blank': [2, 'always'], - 'body-empty': [0, 'never'], - 'body-min-length': [0, 'always', 1], - 'body-case': [0, 'always', 'lower-case'], // Footer 'footer-leading-blank': [2, 'always'], - 'footer-empty': [0, 'never'], - 'footer-max-length': [0, 'always', 72], // Header - 'header-case': [0, 'always', 'lower-case'], 'header-full-stop': [2, 'never', '.'], 'header-max-length': [2, 'always', 72], 'header-min-length': [2, 'always', 1], // Scope / Subject - 'scope-case': [0, 'always', 'lower-case'], - 'subject-case': [0, 'always', 'lower-case'], - 'subject-empty': [0, 'never'], 'subject-full-stop': [2, 'never', '.'], // Signed-off-by 'signed-off-by': [2, 'always', 'Signed-off-by:'], },Note: With
'subject-empty': [0, 'never']disabled, commits without a subject line are allowed. If this is intentional, consider adding a comment explaining why subjects are optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)commitlint.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .pre-commit-config.yaml
🔇 Additional comments (3)
commitlint.config.ts (3)
1-4: LGTM! Correct TypeScript configuration structure.The type import and preset extension follow commitlint best practices.
20-20: Verify the header length limit is intentional.The
header-max-lengthis set to 72 characters, which is more restrictive than the conventional preset default of 100. Since the header includes type, scope, and subject (format:type(scope): subject), this leaves limited space for the actual subject line.Please confirm this is the intended length limit for your project's commit messages. If 72 characters is too restrictive for practical use, consider using 100 or another value that better fits your needs.
29-36: LGTM! Signed-off-by enforcement and documentation link.The
signed-off-byrule correctly enforces the requirement mentioned in the PR objectives, and thehelpUrlprovides helpful documentation for contributors.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)
38-42: LGTM! (Optional: prefer explicit long-form argument)The codespell configuration correctly references
spell.ignorewhich will be present in consuming repositories. The current arg format-I=spell.ignoreworks, but consider the more explicit format for better readability:Based on learnings.
- id: codespell - args: ['-I=spell.ignore'] + args: ['--ignore-words', 'spell.ignore']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)commitlint.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:59:46.902Z
Learnt from: PaulYuuu
PR: avocado-framework/avocado-static-checks#14
File: .pre-commit-config.yaml:30-35
Timestamp: 2025-09-29T03:59:46.902Z
Learning: When .pre-commit-config.yaml is in a submodule, it's correct for it to reference files like 'spell.ignore' that will exist in the consuming repositories, not in the submodule itself.
Applied to files:
.pre-commit-config.yaml
🔇 Additional comments (10)
commitlint.config.ts (3)
1-4: LGTM!The TypeScript import and config initialization follow best practices.
5-31: LGTM!The rules configuration properly enforces commit message standards while maintaining flexibility through selectively disabled rules. The
signed-off-byrequirement aligns with the PR objectives for DCO compliance.
32-36: LGTM!The help URL provides useful guidance for contributors, and the default export is correctly formatted.
.pre-commit-config.yaml (7)
1-2: LGTM!The default hook types and stages are correctly configured to support both pre-commit checks and commit message validation.
4-15: LGTM!The pre-commit hooks configuration is comprehensive and follows best practices. The exclusion of Markdown and reStructuredText files from trailing whitespace checks is appropriate since these formats use trailing spaces for line breaks.
20-24: LGTM!The isort configuration with
--profile=blackensures proper compatibility between import sorting and Black formatting.
43-48: Add TypeScript dependencies for commitlint config.The commitlint hook references
commitlint.config.ts, a TypeScript configuration file that requires transpilation. The currentadditional_dependenciesonly includes@commitlint/config-conventional, missing the TypeScript runtime dependencies.Apply this diff to add the required TypeScript dependencies:
- id: commitlint - additional_dependencies: ['@commitlint/config-conventional'] + additional_dependencies: + - '@commitlint/config-conventional' + - 'typescript' + - 'ts-node' stages: [commit-msg]Likely an incorrect or invalid review comment.
16-19: Gitleaks version is current. v8.28.0 is the latest release, so the pre-commit hook configuration is up-to-date and ready for deployment.
25-28: The Black version 25.9.0 is valid and is the latest release.Black 25.9.0 was released on September 19, 2025, and is confirmed as the current latest version on PyPI. The original concern was based on information with a March 2025 knowledge cutoff, when versions in the 24.x range were current. The version numbering (25 = year 2025, 9 = September) follows Black's established convention.
29-37: Fix the pylint hook configuration incompatibility.The current configuration combines
language: pythonwith a shell script entry (static-checks/check-lint), which is incompatible. Thelanguage: pythoncreates a Python virtual environment and expects the entry to be a Python module or script invocable via the Python interpreter, not an arbitrary shell script.Consider one of these solutions:
Option 1 (Recommended): Use
language: systemif pylint and check-lint script are available in the system PATH:- repo: local hooks: - id: pylint name: pylint entry: static-checks/check-lint - language: python - additional_dependencies: - - pylint==4.0.0 + language: system pass_filenames: false + types: [python]Option 2: Create a Python wrapper script that can be executed by the Python environment:
- repo: local hooks: - id: pylint name: pylint - entry: static-checks/check-lint + entry: python static-checks/check-lint-wrapper.py language: python additional_dependencies: - pylint==4.0.0 pass_filenames: false + types: [python]Option 3: Use
language: scriptwithout additional_dependencies (requires pylint pre-installed):- repo: local hooks: - id: pylint name: pylint entry: static-checks/check-lint - language: python - additional_dependencies: - - pylint==4.0.0 + language: script pass_filenames: false + types: [python]Likely an incorrect or invalid review comment.
PaulYuuu
force-pushed
the
pre-commit
branch
2 times, most recently
from
325bd7a to
8ce0939
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.pre-commit-config.yaml (1)
38-44: Consider using the long-form argument for codespell.The short form
-I=spell.ignoreworks correctly, but the long form--ignore-wordsis more explicit and self-documenting.Apply this diff if you prefer the long form:
- id: codespell - args: ['-I=spell.ignore'] + args: ['--ignore-words', 'spell.ignore'] exclude: 'static-checks/.*'Note: Based on learnings, the
spell.ignorefile will exist in consuming repositories, not in this submodule, which is the correct pattern for shared pre-commit configurations.Based on learnings
check-lint (1)
89-93: Skipping pylint when config is missing is intentional
Most avocado-framework repos don’t include avocado-static-checks.conf, so exiting 0 avoids lint failures in submodules.
Optional: document this in your README or add an env var (e.g. FAIL_ON_MISSING_CONFIG) to enforce a missing-config error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.pre-commit-config.yaml(1 hunks)check-lint(1 hunks)commitlint.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- commitlint.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-29T03:59:46.902Z
Learnt from: PaulYuuu
PR: avocado-framework/avocado-static-checks#14
File: .pre-commit-config.yaml:30-35
Timestamp: 2025-09-29T03:59:46.902Z
Learning: When .pre-commit-config.yaml is in a submodule, it's correct for it to reference files like 'spell.ignore' that will exist in the consuming repositories, not in the submodule itself.
Applied to files:
.pre-commit-config.yaml
🔇 Additional comments (1)
.pre-commit-config.yaml (1)
44-49: LGTM! Commitlint configuration is correct.The commitlint hook is properly configured with:
- The conventional config as an additional dependency
- Correct stage override to
commit-msg(different from the defaultpre-commit)- Appropriate version of the hook
This aligns well with the new
commitlint.config.tsfile mentioned in the PR objectives.
|
Changes:
avocado-vt does not use check-lint, even if we can skip the check via SKIP=pylint env variable in the CI env, but for user, they also have to do this, change the behavior of the script for more user friendly. |
|
Hello @clebergnu @richtja, any concerns of the latest changes? |
This commit centralizes the static checks configuration: - Add .pylintrc with disabled pylint messages and excluded directories from the inspekt lint command (W, R, C, and specific error codes). Also excludes avocado-libs and scripts/github directories. - Add avocado-static-checks.conf to configure the static checks system - Simplify Makefile check target to use a single run-static-checks script instead of multiple individual check commands - Replace inspekt lint usage: inspekt is outdated and no longer in active development, so the configuration has been migrated to use pylint directly via .pylintrc This refactoring makes the static checks configuration more maintainable and easier to update in a single location. Assisted-by: Cursor Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
This commit centralizes the static checks configuration: - Add .pylintrc with disabled pylint messages and excluded directories from the inspekt lint command (W, R, C, and specific error codes). Also excludes avocado-libs and scripts/github directories. - Add avocado-static-checks.conf to configure the static checks system - Simplify Makefile check target to use a single run-static-checks script instead of multiple individual check commands - Replace inspekt lint usage: inspekt is outdated and no longer in active development, so the configuration has been migrated to use pylint directly via .pylintrc This refactoring makes the static checks configuration more maintainable and easier to update in a single location. Assisted-by: Cursor Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
This commit centralizes the static checks configuration: - Add .pylintrc with disabled pylint messages and excluded directories from the inspekt lint command (W, R, C, and specific error codes). Also excludes avocado-libs and scripts/github directories. - Add avocado-static-checks.conf to configure the static checks system - Simplify Makefile check target to use a single run-static-checks script instead of multiple individual check commands - Replace inspekt lint usage: inspekt is outdated and no longer in active development, so the configuration has been migrated to use pylint directly via .pylintrc This refactoring makes the static checks configuration more maintainable and easier to update in a single location. Assisted-by: Cursor Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
This commit centralizes the static checks configuration: - Add .pylintrc with disabled pylint messages and excluded directories from the inspekt lint command (W, R, C, and specific error codes). Also excludes avocado-libs and scripts/github directories. - Add avocado-static-checks.conf to configure the static checks system - Simplify Makefile check target to use a single run-static-checks script instead of multiple individual check commands - Replace inspekt lint usage: inspekt is outdated and no longer in active development, so the configuration has been migrated to use pylint directly via .pylintrc This refactoring makes the static checks configuration more maintainable and easier to update in a single location. Assisted-by: Cursor Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
richtja
left a comment
richtja
left a comment
There was a problem hiding this comment.
Hi @PaulYuuu, thank you for the update, most of your changes LGTM. I would just remove the skip option for lint checks.
You are right that currently avocado-vt uses pylint through inspektor, but IMO the inspektor is quite outdated without any development for many years. Therefore, I introduced avocado-framework/avocado-vt#4260. Which will move avocado-vt from inspektor to static-cheks. Let me know what do you think?
I am okay with keeping the pylint check. For VT, it should also migrate to pylint indeed. FYI, tp-qemu migrated from inspektor to ruff, but in this PR, let me respect the design of avocado-framework. |
- Add comprehensive pre-commit configuration with hooks for: - Code quality checks (black, isort, pylint, codespell) - Security scanning (gitleaks) - Line ending and whitespace fixes - Add commitlint configuration with conventional commit format - Configure signed-off-by trailer requirement for commits Signed-off-by: Yihuang Yu <[email protected]>
github-project-automation
bot
moved this from Review Requested
to Done 113
in Default project
This commit introduces the latest changes in avocado-static-checks repo, which uses pre-commit hooks instead of current scripts solution. Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
2 participants
Summary by CodeRabbit